Skip to content

fix: return actual success from checkIntoMatch and validate relay fragment params#136

Open
Flegma wants to merge 1 commit intomainfrom
audit/380-logic-fixes
Open

fix: return actual success from checkIntoMatch and validate relay fragment params#136
Flegma wants to merge 1 commit intomainfrom
audit/380-logic-fixes

Conversation

@Flegma
Copy link
Copy Markdown
Contributor

@Flegma Flegma commented Mar 30, 2026

Summary

  • checkIntoMatch: Was always returning success: false regardless of outcome. Now returns true when the player's check-in mutation affects rows.
  • match-relay controller: Replaced raw parseInt(fragment) with NestJS ParseIntPipe — returns 400 Bad Request on non-numeric fragment values instead of passing NaN downstream.

Note: The veto condition at line 455 was reviewed and found to be correct (uses && within branches, || between them). No change needed.

Test plan

  • Check into a match as a player — API returns success: true
  • Check into match with invalid state — returns error as before
  • Access relay with valid fragment number — works normally
  • Access relay with non-numeric fragment (e.g., abc) — returns 400

Closes 5stackgg/5stack-panel#380

…gment params

- checkIntoMatch: return success based on affected_rows instead of
  hardcoded false, so clients can confirm check-in succeeded
- match-relay controller: use ParseIntPipe on fragment params to
  validate they are valid integers (returns 400 on invalid input)
  instead of unguarded parseInt which could produce NaN

Closes 5stackgg/5stack-panel#380
@Res() response: Response,
) {
this.matchRelayService.getStart(response, matchId, parseInt(fragment));
this.matchRelayService.getStart(response, matchId, fragment);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to test this, it may actually be a string. test via enabling relay

@Flegma
Copy link
Copy Markdown
Contributor Author

Flegma commented Mar 31, 2026

Checked the relay service — getStart, getFragment, and postField all accept fragmentIndex: number, so ParseIntPipe is type-compatible.

The difference: before, a non-numeric fragment would pass NaN to the service silently. Now it returns 400 Bad Request instead.

Happy to test with relay enabled, or revert to parseInt with an explicit NaN check if you prefer a safer rollout. Let me know.

@lukepolo
Copy link
Copy Markdown
Contributor

yah we will need to actually test this live. cause just cause the type says its an int,doesnt mean its an int :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[API] Match state transition allows skipping veto, checkIntoMatch hardcoded false, fragment NaN

2 participants